vfs: inline READDIRPLUS attributes the tree can size cheaply - #10
Merged
Andrew Gazelka (andrewgazelka) merged 1 commit intoAug 1, 2026
Merged
Conversation
Andrew Gazelka (andrewgazelka)
force-pushed
the
vfs-readdirplus-attributes
branch
from
August 1, 2026 02:36
8699484 to
c77a271
Compare
Andrew Gazelka (andrewgazelka)
changed the base branch from
vfs-writable-overlay
to
ix-patched
August 1, 2026 02:37
Andrew Gazelka (andrewgazelka)
marked this pull request as ready for review
August 1, 2026 02:37
Andrew Gazelka (andrewgazelka)
force-pushed
the
vfs-readdirplus-attributes
branch
from
August 1, 2026 02:41
c77a271 to
c5f3831
Compare
READDIRPLUS returned no attributes for any entry, so a client spent one LOOKUP per entry it cared about. On a cold walk of the ix tree that was 22,477 LOOKUPs out of 31,083 RPCs, 72% of the whole walk. The comment justifying it was correct when written and its premise has since expired. Sizing a file used to mean reading it, so filling these in would have turned listing a directory into reading every file in it. `Backend::file_size` now answers from the git object header without inflating the object, and `getattr` already takes that path. Inline attributes for directories and files, which are cheap to size. Keep them out for conflicted paths, whose size still needs the sides materialized, and for symlinks, whose length still needs the target. NFSv3 makes the field optional per entry, so those two cost a LOOKUP each and nothing else. Split the fattr3 constants into FattrContext so the iterator can size entries without borrowing the tree, and give DirIterator a next_entry that skips the attribute work, so plain READDIR pays nothing for a field it would discard. Measured against the same tree and revision, cold `find -type f`: 31,083 RPCs to 9,931, a 3.13x cut, with LOOKUPs down 17x from 22,477 to 1,320. A listing-only walk that never stats goes 29,873 to 8,683, because without inline attributes the client reports DT_UNKNOWN and a portable walker has to probe every entry anyway.
Andrew Gazelka (andrewgazelka)
force-pushed
the
vfs-readdirplus-attributes
branch
from
August 1, 2026 03:24
c5f3831 to
b0829f9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
READDIRPLUS returned no attributes for any entry, so a client spent one LOOKUP per entry it cared about. This fills them in where the tree can size an entry cheaply.
The cost being paid today
On a cold
find -type fover a real tree of 16,894 files, 4,302 directories and 21,196 entries, the walk issued 19,242 LOOKUPs at 1.026 per directory entry, 70.6% of every RPC it made. That is the readdir-driven walk a build actually performs, not a synthetic pure-stat sweep.What the lookups were, which is not what it first looked like
They were not redundant. On the original tree, 18,718 lookups against 18,721 entries is 0.9998 per entry — exactly one per distinct path component, none repeated. The directory vnode cache was already perfect and there was no duplication to remove.
An earlier framing of "1.27 lookups per stat" came from dividing by stats rather than by entries; the excess over 1.0 was simply the directory components, each resolved once. Recording that because it changes what the fix is: the only way to remove these lookups was to stop needing them, which is what inlining attributes does.
Why this is not a trade
A LOOKUP reply already computes exactly these attributes. Measured with
proc_pid_rusageacross a cold sweep, an entire LOOKUP RPC — XDR decode, protocol, handler,size(),file_size(), XDR encode — costs 0.616 us of server CPU. Filling a READDIRPLUS entry therefore costs strictly less than that.Under 0.6 us of local work against a 63.9 us round trip. About 100x favourable per entry.
Ceiling, stated as a ceiling
Before measuring, the predicted ceiling was 27,244 RPCs down to roughly 8,000, taking about 1.23 s off a 2.81 s cold walk, about 1.8x. Not "faster" — that is the most the change can be worth.
Measured
Same repo, same revision, two mountpoints, one binary each, cold walk on each. Control 23 RPCs / 20 s idle.
find -type fRepeat on a fresh cold mount: 9937 RPCs, Lookup 1320 exactly.
Total RPCs 3.13x fewer. LOOKUPs 17.0x fewer, 94.1% eliminated. Lookups per entry 1.060 -> 0.062.
Wall clock understates it: the patched server was a debug build and still beat an optimized one. The ratio is the number to read, not the time.
What tree these numbers are from. Both arms mounted
~/.config/nixat revision0fc2d7cd0c22— the nix-config repo, 16,894 files and 4,302 directories. Earlier drafts of this description called it "the ix tree", which was wrong: on this machine ix is a derived view served from a separateix-viewref, and a mount taken with-r maingets nix-config. The A/B is unaffected, because both the patched and unpatched runs mounted the same source and the file and directory counts were verified equal between them, so every ratio here stands. Only the label was wrong, and it is corrected rather than quietly dropped because someone will otherwise re-derive these numbers against the wrong repo.A first attempt at this A/B was discarded — the repo had grown from 14,720 to 16,894 files between mounting the baseline and building the patched binary, so it was comparing two different trees.
The client uses them, on macOS
The macOS NFSv3 client uses inline attributes and does not re-LOOKUP. Measured, and it was not a given. Not tested on Linux, so this is macOS-verified rather than verified.
The new floor, and why this stops here
Of the 9,934 RPCs remaining, 8,611 (87%) are RdirPlus + Access, which is 2.0 RPCs per directory across 4,302 directories — NFSv3 directory traversal, not something a server change removes.
Eliminating every one of the 1,320 residual LOOKUPs would buy a further 1.15x. I did not determine what those 1,320 are; they are not symlinks, of which there are only 29. I stopped because of that ceiling, not because the question was answered. Named here as a bounded loose thread so nobody has to rediscover it.
The honest cost
A caller that lists a directory and looks at nothing — no
stat, no recursion, names only — now receives a sizedfattr3per entry it did not ask for. That is precisely what the original comment was protecting, and it is a real cost.Measured on the largest directory in the tree, 955 entries, listing only. The extra cost is 0.134 us of server CPU per entry, and 2 additional READDIRPLUS round trips for one cold listing (3 -> 5) because sized entries are larger and fit fewer per reply. Together about 256 us for a cold 955-entry listing. Since each avoided LOOKUP is worth 63.9 us, the change breaks even if the caller looks at 4 of those 955 entries, and
ls -llooks at all of them.The recursive listing case does not pay this at all — it gets faster. A walk that never stats went 29,873 RPCs to 8,683, because omitting attributes leaves
d_typeunset: the client reportsDT_UNKNOWNfor every entry, so a portable walker must probe each one and generates the LOOKUP anyway. Verified directly — before:UNKNOWN=58 DIR=0 REG=0; after:UNKNOWN=0 DIR=30 REG=28. So today's default optimises the rarer case, and the server also burns less CPU after the change on that walk (10,121 us vs 12,445 us) because 21,195 whole RPCs stop arriving.Scope: which workloads gain, and which do not
This helps traversal. It does very little for a package manager, and that is worth stating up front because the package-manager case is the one that prompted the filesystem work.
Another measurement of this same change found it worthless on
bun install: getattr 3,397,989 -> 3,374,005 and total operations 5,430,450 -> 5,394,137, both about -0.7% and inside noise. That result is correct. So is the 3.13x above. They are different workloads, and the boundary between them is measurable.Both runs below use the same instrument, the server's own
JJ_VFS_STATSop census, against the same mount.find -type fbun installbun installdoes 91x more path resolution per directory it lists. It issues 20.6 lookups for every file it creates — and a listing-derived lookup can happen at most once per entry, so anything above 1 per file has to be resolution of paths bun constructed rather than paths it discovered. That is a package manager replaying a lockfile: it already knows every path it wants.Note it does not skip readdir entirely — 4,325 of them, comparable to the walker. It lists directories and then resolves tens of thousands of paths that were never in a listing.
The ceiling this puts on the change, for that workload. Attributes can only ever save a lookup for an entry that appeared in a listing. bun created 5,828 files and 569 directories, so at most 6,397 entries were ever listable — 5.3% of its lookups and 0.80% of its 797,696 total operations. The independently observed -0.67% sits right at that predicted ceiling. The two measurements agree; one predicts the other.
One trap in that census, for whoever reads it next. The
JJ_VFS_STATSgetattrcounter records internal calls, not client RPCs: it read 494,589 while NFSGETATTRRPCs over the same run were 37,169, because the server callsgetattrinternally to fill replies forlookup,createand the rest. It is sound for comparing two runs of the same workload, which is what it is for, but it is not a count of what the client asked for, and comparing it againstnfsstatoutput will produce a false conclusion.So: large win for anything that lists a tree and then looks at what it found —
find,fts(3), ripgrep,fd, git, ninja, and a Nix source walk. Approximately nothing for a package manager replaying a lockfile. Traversal is most of what a build does outside the package manager, which is why this is still worth landing, but it should not be advertised as speeding upbun install.What changed
DirIteratorinlinesfattr3for directories, whose size is a constant, and for files, whose length comes from the git object header.Two cases keep
None, each named with its own comment because they are where the old reasoning still holds: a conflicted path, which has no content until its sides are materialized, and a symlink, whose length needs its target fetched. The symlink case is a second live instance of the rule this PR retires, found while implementing it. NFSv3 makes the field optional per entry, so each costs one LOOKUP and nothing else.The comment was rewritten rather than deleted. It records what the old trade was, why it was right when written, and what changed underneath it, so nobody re-adds the old behaviour and nobody reads the old decision as carelessness.
FattrContextis split out ofNfsTreeso the iterator can size entries without borrowing the tree, andDirIterator::next_entrylets plain READDIR skip the attribute work entirely — no effect on macOS, which only ever issues READDIRPLUS, but Linux clients do issue plain READDIR.Tests
Two added, both watched to fail before being trusted:
name_attributesback toNonefailstest_nfs_readdirplus_inlines_cheaply_sized_attributestest_nfs_readdirplus_omits_attributes_for_a_conflictFull
jj-vfssuite 27/27 green.cargo clippy -p jj-vfs --all-features --all-targetsclean.The first push of this branch failed
check (rustfmt), and the reason is worthrecording rather than quietly amending away. I verified with
cargo testandcargo clippyand reported that as passing, but the repo's gate also runscargo +nightly fmt --all -- --check, which I had not run. Two defects: a strayblank line in
nfs.rsleft by splitting an impl block, and a hand-wrapped stringin
test_nfs.rsthat fits insidemax_width = 100and so is unwrapped byformat_strings = true. A pass on some of the checks is not a pass on thechecks. Fixed with
cargo +nightly fmt --all, confirmed only those two fileschanged, and re-ran tests and clippy afterwards rather than assuming a
formatting change was inert.
Not a mount option
Ruled out before writing any code.
rdirplusis both the documented default and present on the live mounts, and the client already uses it exclusively — lifetime counters showReaddir=0againstRdirPlus=248189.mount_nfs(8)even namesfind . -type fas the case it should help. The client was asking correctly; the server was declining.Rebased onto #9
#9 landed as
34ed0a3e3and touched the same entry builder, so this was rebased onto it rather than the other way round. #9 renamedNfsTree's snapshot field totree: Arc<OverlayTree>, moved the mode bits onto the tree, replaced the mount-wide mtime with per-entry timestamps, and added anext_cookietoDirIterator— this change is adapted to all of that, andFattrContextnow carries the tree because bothmode_bitsandgetattrlive there.The conflict was resolved by taking the new base verbatim and re-applying the change, rather than hand-editing conflict markers, so what is here is a clean re-port rather than a merge artifact. Every check was re-run on the rebased artifact — including breaking both guards again to confirm they still fire — rather than assuming the earlier verification carried.
Nothing in the 101 added lines is platform-conditional: no
cfg(, nolibc::, no errno comparison. Checked against a denominator, since those patterns do appear elsewhere in the crate (sys.rs,snapshot.rs,fuse.rs).Background and the full measurement log: ENG-11746.